Allowing to define variable line heights #233110
Conversation
Decorations may affect line wrapping. For example, they make make the text bold, or increase or decrease font size. This wasn’t originally taken into account to calculate the the break offsets.
aiday-mar
changed the title
| @@ -97,6 +97,7 @@ export class LinesLayout { | |||
| private _lineHeight: number; | |||
| private _paddingTop: number; | |||
| private _paddingBottom: number; | |||
| private _specialLineHeights = new Map<number, number>(); | |||
There was a problem hiding this comment.
Let's implement this using a more performant data structure which makes things slow only when the feature is exercised. For example, for a file with 100K lines, getting the top position for line 99K will involve querying this _specialLineHeights map 99K times.
It also seems to me that we're not quite keeping this field up to date correctly. If I set a line height of 40 on a line 1000 and then press enter on the first line, will the _specialLineHeights be updated correctly to reflect the edit I did?
There was a problem hiding this comment.
Thanks for the comment. The way that the line height is currently calculated is:
private _linesHeight(_untilLineNumber?: number): number {
const untilLineNumber = _untilLineNumber ?? this._lineCount;
let specialLinesHeight = 0;
let numberOfSpecialLines = 0;
this._specialLineHeights.forEach((height, lineNumber) => {
if (lineNumber <= untilLineNumber) {
specialLinesHeight += height;
numberOfSpecialLines++;
}
});
const nonSpecialLinesHeight = (untilLineNumber - numberOfSpecialLines) * this._lineHeight;
return specialLinesHeight + nonSpecialLinesHeight;
}
Supposing we want to query the height of line 99K and between line 1 and line 99K there are for example 5 special line heights, then we will query the map only 5 times in the code above, as we iterate over the special line heights and add the line heights of the lines below our line of interest. Please let me know if I am misunderstanding what you mean, so I can rework the code.
Concerning the second point, the map is updated when the onDidChangeSpecialLineHeight event is fired. This event is fired when we detect that the line height decoration changes. The decorations change event is also fired when edits are applied in the editor. From this I believe the special line heights should be synchronized.
| private _getLineHeightForLine(lineNumber: number): number | null { | ||
| const startOffset = this._buffer.getOffsetAt(lineNumber, 1); | ||
| const endOffset = startOffset + this._buffer.getLineLength(lineNumber); | ||
| return this._decorationsTree.getLineHeightInInterval(this, startOffset, endOffset, 0); | ||
| } |
There was a problem hiding this comment.
There is something not quite correct here. Line heights don't work quite like injected text, because we should allow for line heights of the same line to be different across different editors. For example, if a particular text model is shown in editor1 and editor2, we should allow that a particular line has a particular height in editor1 and a different height in editor2, since line height is a presentation option (which is arguably different than injected text). So I think the decoration ownerId should play a part in this method call, not quite sure who calls it.
fixes #147067